Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Async streaming iOS #599

Merged
merged 3 commits into from
Feb 28, 2025
Merged

Async streaming iOS #599

merged 3 commits into from
Feb 28, 2025

Conversation

cehan-Chloe
Copy link
Contributor

@cehan-Chloe cehan-Chloe commented Feb 21, 2025

core update can be found here

demo app screenshot:
image
image

Change Type (required)

Indicate the type of change your pull request is:

  • patch
  • minor
  • major
  • N/A

Does your PR have any documentation updates?

doc update in core pr

  • Updated docs
  • No Update needed
  • Unable to update docs

Release Notes

Player streaming enhancement for iOS

Changes

  • Update iOS PlayerUIDemo with Chat-message asset basic
  • Update async-node-plugin and reference-assets-plugin tests

Copy link

codecov bot commented Feb 21, 2025

Codecov Report

Attention: Patch coverage is 98.28571% with 6 lines in your changes missing coverage. Please review.

Project coverage is 89.87%. Comparing base (7828cb2) to head (60e5c52).
Report is 14 commits behind head on async-streaming-core.

Files with missing lines Patch % Lines
...ins/async-node/ios/Tests/AsynNodePluginTests.swift 98.25% 6 Missing ⚠️
Additional details and impacted files
@@                   Coverage Diff                    @@
##           async-streaming-core     #599      +/-   ##
========================================================
+ Coverage                 89.72%   89.87%   +0.14%     
========================================================
  Files                       334      335       +1     
  Lines                     19923    20273     +350     
  Branches                   1956     1956              
========================================================
+ Hits                      17876    18220     +344     
- Misses                     2033     2039       +6     
  Partials                     14       14              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@cehan-Chloe cehan-Chloe marked this pull request as ready for review February 21, 2025 15:41
@cehan-Chloe cehan-Chloe marked this pull request as draft February 21, 2025 16:29
@cehan-Chloe cehan-Chloe marked this pull request as ready for review February 21, 2025 16:29
}

let asyncNodePluginPlugin = AsyncNodePluginPlugin()
let plugin = AsyncNodePlugin(plugins: [asyncNodePluginPlugin], resolve)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

out of scope for the PR, but this constructor API is inconsistent across platforms

XCTAssertEqual(expectedNode1Text, "chat message")

wait(for: [textExpectation2], timeout: 5)
XCTAssertEqual(expectedNode2Text, "chained chat message")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥


wait(for: [textExpectation2], timeout: 5)
XCTAssertEqual(expectedNode2Text, "chained chat message")
}
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add additional test using the callback mechanism using chat message similar to

func testHandleMultipleUpdatesThroughCallback() {

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ty! added new callback tests for chatmessage

@cehan-Chloe cehan-Chloe requested a review from nancywu1 February 26, 2025 17:58
Copy link
Contributor

@nancywu1 nancywu1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, thanks for adding the additional test!

@cehan-Chloe cehan-Chloe merged commit b116d7d into async-streaming-core Feb 28, 2025
11 checks passed
@cehan-Chloe cehan-Chloe deleted the async-streaming-ios branch February 28, 2025 15:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants